Skip to content

Simplify OTEL metrics config to mirror traces pattern#68393

Open
dsuhinin wants to merge 29 commits into
apache:mainfrom
dsuhinin:dsuhinin/simplify-otel-logger-config
Open

Simplify OTEL metrics config to mirror traces pattern#68393
dsuhinin wants to merge 29 commits into
apache:mainfrom
dsuhinin:dsuhinin/simplify-otel-logger-config

Conversation

@dsuhinin

@dsuhinin dsuhinin commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Mirror airflow_shared.observability.traces.configure_otel's shape in the metrics module so both halves of the OTel config story have one entry point, one conf-driven setup function, and the same backcompat-bridging contract.

What changes

  • New: airflow_shared.observability.metrics.configure_otel(conf) — a single conf-driven entry point that bridges deprecated Airflow configs (metrics.otel_host / otel_port / otel_ssl_active / otel_service / otel_interval_milliseconds) into the standard OTel env vars, loads the exporter via
    the SDK's entry-point mechanism, installs the global MeterProvider, and returns a SafeOtelLogger for the Stats factory.
  • Removed: the 11-parameter get_otel_logger(host=, port=, ssl_active=, prefix=, conf_interval=, debug=, ...) function and the helper get_otel_data_exporter that duplicated the OTel SDK's own exporter dispatch.
  • Removed: the 26-line shim file in airflow-core and the near-identical one in task-sdk. Both stats_utils.py files now wire directly to configure_otel.
  • Removed: otel_env_config.py (entirely unused after the bridging consolidated into _get_backcompat_config).

OTel environment variables take precedence over the deprecated Airflow configs in every code path, matching the rule already used in traces/configure_otel. IPv6 host bracketing in the bridged URL is preserved via _format_url_host.

Why

Before: 11-parameter shared function + 4-parameter exporter helper + a 26-line shim duplicated across two distributions + an unused env-config dataclass module. Net ~700 lines for what is, conceptually, "read conf, bridge to env, install provider."

After: one configure_otel(conf) function that follows the same shape as the existing traces module. ~225 fewer lines net. New developers who learn one module understand the other immediately.

Verified

  • Unit tests: 41/41 pass (8 parametrize cases for the new bridging contract; protocol-dispatch sub-tests dropped because they exercised SDK internals, not our code).
  • End-to-end: metrics emitted via the deprecated AIRFLOW__METRICS__OTEL_* configs (no env vars set) arrived at a real OTel collector — proving the bridging works end-to-end through the OTel SDK pipeline.

Important

🛠️ Maintainer triage note for @dsuhinin · by @potiuk · 2026-07-15 16:20 UTC

The ready for maintainer review label was removed — the next move is yours (see our Pull Request quality criteria):

  • Merge conflicts — rebase on main. See docs.

The ball is in your court — you've been assigned to this PR. Rebase on main, resolve conflicts, then mark it Ready for review again.

Automated triage — may be imperfect; a maintainer takes the next look.

Comment thread shared/observability/src/airflow_shared/observability/metrics/__init__.py Outdated
Comment thread airflow-core/src/airflow/observability/metrics/otel_logger.py
Comment thread airflow-core/src/airflow/observability/metrics/stats_utils.py
Comment thread shared/observability/src/airflow_shared/observability/metrics/otel_logger.py Outdated
@dsuhinin
dsuhinin marked this pull request as ready for review June 15, 2026 12:50
@dsuhinin
dsuhinin requested a review from jason810496 as a code owner June 17, 2026 15:10

@xBis7 xBis7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks!

@potiuk potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Jun 22, 2026
@potiuk potiuk removed the ready for maintainer review Set after triaging when all criteria pass. label Jul 15, 2026
@potiuk
potiuk marked this pull request as draft July 20, 2026 11:54
@potiuk

potiuk commented Jul 20, 2026

Copy link
Copy Markdown
Member

@dsuhinin This PR has been converted to draft because it does not yet meet our Pull Request quality criteria.

Issues found:

  • Merge conflicts: This PR has merge conflicts with main and cannot be merged as-is. Rebase locally and resolve them: git fetch upstream main && git rebase upstream/main, resolve the conflicts, then force-push.

Note: Your branch is 671 commits behind main. Some check failures may be caused by changes in the base branch rather than by your PR. Please rebase your branch and push again to get up-to-date CI results.

What to do next:

  • Fix each issue listed above.
  • Make sure static checks pass locally (prek run --from-ref main --stage pre-commit).
  • Mark the PR as "Ready for review" when you're done.

Converting a PR to draft is not a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the Airflow Slack.


Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you.

…tel-logger-config

# Conflicts:
#	shared/observability/tests/observability/test_traces.py
@dsuhinin
dsuhinin marked this pull request as ready for review July 22, 2026 07:22
@dsuhinin
dsuhinin requested a review from Lee-W as a code owner July 22, 2026 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants